Skip to content

Conversation

@Fireworks
Copy link
Contributor

Sorry this PR is messy, and might be hard to follow. Really just pay attention to changes in Poll.java and Connection.java. Make sure you re-disable any bot listeners you do not want active.

Upgrade to Pircxbot 2.1 if you are going to keep the new server connection strategy with addServer and setServerPassword. Otherwise keep the current strategy of setServer as it is now.

  • Adds basic "Poll" functionality
    • !poll option1 option2 option3 (space separated options)
    • !poll to end the poll
  • Remove a bunch of junk files that shouldn't be tracked in git

@amccarthy1
Copy link

I tested this out and looked at the code and it turns out anyone can vote any number of times, might want to add a HashSet of user IDs or something to make sure people can only vote once.

@Fireworks
Copy link
Contributor Author

@amccarthy1 Hah, thanks. I wrote this late at night... what a huge oversight. I'll do something like that.

src/Poll.java Outdated
}

if (voteKey != 0) {
votes[voteKey-1] = votes[voteKey-1] + 1;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This throws an ArrayIndexOutOfBoundsException for negative values or values greater than the number of options (which happens quite a lot in twitch chat). The exception is caught and not logged or anything but you should still probably do bounds checking to handle that a little more gracefully.

@Fireworks
Copy link
Contributor Author

@amccarthy1 Fixed in latest commit. Went with another HashMap for the user votes, similar to what I am doing in Predictions (except storing user UUIDs instead of Nick as key).

Also changed the input matching to split on commas, which should make it easier to do something like: !poll super mario 64, sunshine, sims

Let me know if you see any other issues, and appreciate the review. Should be a lot better now, though I'm still not testing thoroughly :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants